docs: document summary definitions on custom attestation types - #384
docs: document summary definitions on custom attestation types#384dangrondahl wants to merge 2 commits into
Conversation
Custom attestation types can define a summary: an ordered list of named jq expressions that Kosli renders as labeled rows on the attestation detail page instead of only raw JSON. - getting_started/attestations.md: new "Summaries" subsection in the Custom accordion, covering the CLI flags and the rendering behavior (versioning, render-time evaluation, N/A, URL links, array payloads). - tutorials/attest_custom.md: the tutorial type is now created with --summary entries so the walkthrough ends on a readable summary. - administration/managing_custom_attestation_types/overview.md: summary added to the "Each type can include" list, plus a Terraform example. Refs #364
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
|
Claude finished @dangrondahl's task in 2m 49s —— View job Doc review — PR #384
Re-reviewed after Verified
FindingsImprovement
Suggestion
Not verified
Overall: ready to merge once finding 1 is settled. The diff also quietly fixes two pre-existing copy-paste bugs in |
| - A **summary** (optional) — ordered, labeled jq expressions that Kosli renders as rows on the attestation detail page | ||
|
|
||
| At least one of the two must be provided. | ||
| At least one of the schema and the jq rules must be provided. The summary is independent of both: |
There was a problem hiding this comment.
Suggestion — "one of the schema and the jq rules" reads awkwardly.
| At least one of the schema and the jq rules must be provided. The summary is independent of both: | |
| At least one of the schema or the jq rules must be provided. The summary is independent of both: |
There was a problem hiding this comment.
Applied as suggested.
- Move the summaries content out of the collapsed "Custom" accordion into a top-level "Summarizing custom attestations" section. Verified with headless Chrome that a cold load of the in-accordion anchor left every accordion at aria-expanded="false", so the deep link both cross-links rely on landed on hidden content. Also fixes the ####-after-## skip. - Fix the versioning contradiction: attestation.py resolves get_versioned_type(self.type_version) before evaluating, so a rendered attestation uses the type version it was reported against. Both pages now say that, and the Terraform note scopes the fallback to the new version. - Narrow the URL claim: link rendering requires a string value with an http/https scheme (kosli-dev/server#6428 hardening), not "any valid URL". - Trim the tutorial paragraph to one sentence plus one link, and drop "minimal" now that the type carries summary entries. - "At least one of the schema or the jq rules" (was "and"). - Add \ line continuations to the two command blocks in the Custom accordion, and note that re-running create updates an existing type. Refs #364
|
All five inline threads answered and pushed (b5e...see latest commit). Replying here to the one finding that had no inline anchor, plus what came out of verifying the rest. The two substantive items were both real. The versioning contradiction resolved in favor of the The anchor concern was not a maybe — a headless-Chrome load of Checking those also caught something the review didn't: "a value that is a valid URL renders as a link" was too broad. The hardening commit on kosli-dev/server#6428 requires a string value with an On the "reference material in a getting-started page" finding — I mostly disagree, and changed nothing for it. Of the three overlaps named: URL-as-link is not in The drift risk is worth naming, though: the generated CLI page is regenerated from the CLI repo, so if the flag semantics change the hand-written section won't follow. That's an argument for keeping the flag mechanics thin here rather than for trimming the behavior, which is what I've done. Also noted and agreed on the British "labelled" in the generated pages — upstream fix, out of scope here. |
| than as a broken detail page later. | ||
| - An expression that returns `null`, or that fails against a particular attestation's data, renders | ||
| as `N/A`. The rest of the summary still renders. | ||
| - A value that is a string beginning with `http://` or `https://` renders as a clickable link, as |
There was a problem hiding this comment.
Improvement — three different descriptions of the same linkification rule.
This says a value linkifies when it's "a string beginning with http:// or https://", and asserts it works "as annotation values do" — but line 246 of this same file says annotation values linkify when they are "valid URLs", and terraform-reference/resources/custom_attestation_type.mdx:189 says "values that are valid URLs render as links". overview.md:83 repeats the narrow phrasing.
The two rules differ for real inputs (ftp://…, mailto:…, a bare example.com), so one of them is wrong. Pick the accurate one and use it in all three places:
- if the scheme check is the real behavior, line 246 and the Terraform provider description need updating (the latter upstream in the provider repo, since that page is generated);
- if "valid URL" is the real behavior, this line and
overview.md:83should say that.
| } | ||
| ``` | ||
|
|
||
| ### With a summary |
There was a problem hiding this comment.
Suggestion — this heading reads as a fourth mutually-exclusive option.
The three preceding ### headings under Create a custom attestation type enumerate the alternatives of one choice: With schema and jq rules / With jq rules only / With schema only. With a summary is orthogonal — it combines with any of the three, and the example below in fact pairs it with jq rules only.
The body text at line 18-21 makes the independence clear, but a reader scanning the right-hand ToC sees four peers and infers a fourth alternative. Consider ### Adding a summary (verb phrase breaks the "With …" series), or promoting it to its own ## after the three shape variants.
Closes #364
What
Documents
summaryon custom attestation types — the ordered list of named jq expressions that Kosli renders as labeled rows on the attestation detail page instead of only raw JSON.The issue was filed when
summarywas API-only and recommended holding back the authoring instructions. Both surfaces have since shipped (--summary/--summary-jsononkosli create attestation-type, and thesummaryattribute onkosli_custom_attestation_type), so all four open tasks are covered.Changes
getting_started/attestations.md— new#### Summariessubsection in the Custom accordion: what a summary is, the--summaryCLI flags, the JSON form, and the behavior worth knowing (part of the versioned type definition, evaluated at display time, invalid jq rejected at creation,N/Afallback, URLs render as links, one summary group per element for array payloads).tutorials/attest_custom.md— step 1 now creates thecoverage-reporttype with--summary "Coverage=.coverage"/--summary "Tool=.tool", so the walkthrough ends on a readable summary rather than raw JSON.administration/managing_custom_attestation_types/overview.md—summaryadded to the "Each type can include" list (and the "at least one of the two" sentence reworded accordingly), plus a With a summary Terraform example usingjsonencode()and thefile()variant.No new pages, so no
config/navigation.jsonchange.Verification
mint broken-links— the only reported link is the pre-existing/getting_started/service-accountsintutorials/working_with_controls.mdx, untouched here.mint dev: they compile, and the new#summariesheading inside the accordion does get an anchor (id="summaries"), which the tutorial and Terraform page link to.